Repository navigation
refactor: move internal parts and utils from select to listbox - #5204
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5204 +/- ##
==========================================
+ Coverage 97.76% 97.82% +0.06%
==========================================
Files 1003 1005 +2
Lines 32572 32571 -1
Branches 12109 12094 -15
==========================================
+ Hits 31843 31864 +21
+ Misses 723 701 -22
Partials 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
amanabiy
changed the title
amanabiy
changed the title
amanabiy
changed the title
amanabiy
changed the title
| const { inGroupIndex, groupIndex, throughIndex } = getTestOptionIndexes(option) || {}; | ||
| return { | ||
| ...optionProps, | ||
| hasCheckbox: checkboxes, |
There was a problem hiding this comment.
hasCheckbox is not needed anymore because on last PR I introduced selectionType: 'single' | 'mult'. The multi row always draws the checkbox and the single row never does.
Moves useNativeSearch from select/utils to internal/components/listbox as useTypeAhead, and renames its useInteractiveGroups option to interactiveGroups. No behavior change.
Merges select/parts/item.tsx and select/parts/multiselect-item.tsx into internal/components/listbox/parts/item.tsx. The row is chosen with selectionType: 'single' renders today's Select row, 'multi' renders today's Multiselect row (checkbox, interactive groups, select-all, afterHeader). render-options passes selectionType instead of picking a component, and the hasCheckbox/checkboxes plumbing is removed because Select never rendered checkboxes and Multiselect always did. Styles still come from select/parts/styles.scss, so class names and test-utils selectors are unchanged. Dev pages: item.permutations maps hasCheckbox to selectionType, and list-with-sticky-item uses selectionType="multi" (it now shows the checkbox every multi row renders).
Moves select/parts/plain-list.tsx (PlainList and SelectListProps) to internal/components/listbox/parts. Select, Multiselect, embedded Multiselect and VirtualList import it from there. No behavior change.
After the checkboxes input was removed, "with keyboard" made the same call with the same expectation as "should return props for the first child in the first group".
864c388 to
33cfa99
Compare
33cfa99 to
8875ae1
Compare
| if (isMulti) { | ||
| return ( | ||
| <SelectableItem |
There was a problem hiding this comment.
I'd like to keep it as it is for this PR. The two paths are the existing Select and Multiselect rows moved into one file without changes, which keeps the migration easy to review and shows Select and Multiselect render the same as before. Merging them into one shared SelectableItem would change more code here and make the PR harder to review. I'll do it in a follow-up PR.
| }); | ||
| }); | ||
|
|
||
| test('should return props for the first child in the first group with checkboxes and keyboard', () => { |
There was a problem hiding this comment.
This test became a duplicate. Its only difference from the first test was the checkboxes option. After merging the Select and Multiselect rows into one, whether a row shows a checkbox depends on whether it's a Select or Multiselect row, so getItemProps no longer takes that option. Without it, the two tests were identical, so I kept one. Here is the first test test
|
|
||
| describe('useNativeSearch', () => { | ||
| const hook = renderHook(useNativeSearch, { | ||
| describe('useTypeAhead', () => { |
There was a problem hiding this comment.
It's the same feature that used to be called useNativeSearch: when the list has focus, typing letters jumps to the first option that starts with them, like in a native . I renamed it because "type-ahead" is what the ARIA listbox pattern calls it: (https://www.w3.org/WAI/ARIA/apg/patterns/listbox/#keyboardinteraction).
Description
This is a follow up of #5179
Moves Select's option row, type-ahead hook and plain list into
src/internal/components/listbox/and has Select and Multiselect use them, with no behavior change.Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.